Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[pbh] [aclorch] Fixed a bug causes by updating the flow-counter value for the PBH rule #2226

Merged
merged 5 commits into from
Apr 23, 2022
Merged

[pbh] [aclorch] Fixed a bug causes by updating the flow-counter value for the PBH rule #2226

merged 5 commits into from
Apr 23, 2022

Conversation

vadymhlushko-mlnx
Copy link
Contributor

@vadymhlushko-mlnx vadymhlushko-mlnx commented Apr 8, 2022 •
edited by liat-grozovik
Loading

What I did
Added the register and deregister functions for the flex counter before updating counters for the ACL rule.

Why I did it
Overcome SAI error in case of flow counters disabled and then enabled.

For example:

  1. Show PBH rules:
root@sonic:/home/admin# show pbh rule
TABLE      RULE             PRIORITY    MATCH                                 HASH        ACTION         COUNTER
---------  ---------------  ----------  ------------------------------------  ----------  -------------  ---------
pbh_table  nvgre_ipv4_ipv4  2           gre_key:           0x2500/0xffffff00  inner_hash  SET_ECMP_HASH  ENABLED
                                        ether_type:        0x0800
                                        ip_protocol:       0x2f
                                        inner_ether_type:  0x0800
  1. Update PBH rule:
root@sonic:/home/admin# config pbh rule update field set pbh_table nvgre_ipv4_ipv4  --flow-counter DISABLED
root@sonic:/home/admin# config pbh rule update field set pbh_table nvgre_ipv4_ipv4  --flow-counter ENABLED
  1. Observed behavior:
NOTICE syncd#SDK: [SAI_UTILS.NOTICE] mlnx_sai_utils.c[1833]- set_dispatch_attrib_handler: Set ACTION_COUNTER, key:ACL Entry [1], val:NULL,(0:0),0,0000,0
NOTICE syncd#SDK: [SAI_ACL.NOTICE] mlnx_sai_acl.c[14326]- mlnx_delete_acl_counter: Delete ACL Counter ACL Counter [311296] type (byte:true, packet:true)
NOTICE swss#orchagent: :- updateRule: Successfully updated ACL rule nvgre_ipv4_ipv4 in table pbh_table
NOTICE swss#orchagent: :- updatePbhRule: Updated PBH rule(pbh_table|nvgre_ipv4_ipv4) in SAI
ERR syncd#SDK: [GBIN_ALLOCATOR.ERR] LID 38 doesn't exist!
ERR syncd#SDK: [FLOW_COUNTER.ERR] spectrum_flow_counter_lock: cm_lock failed [lid=38, flow_counter_id=311296]
ERR syncd#SDK: [FLOW_COUNTER.ERR] spectrum_flow_counter_get: spectrum_flow_counter_lock failed [cntr_id=311296]
ERR syncd#SDK: [SAI_ACL.ERR] mlnx_sai_acl.c[13776]- mlnx_acl_counter_get:  Failure to get counter in SDK - Entry Not Found
ERR syncd#SDK: [SAI_UTILS.ERR] mlnx_sai_utils.c[1931]- get_dispatch_attribs_handler: Failed getting attrib SAI_ACL_COUNTER_ATTR_BYTES
ERR syncd#SDK: [SAI_UTILS.ERR] mlnx_sai_utils.c[2068]- sai_get_attributes: Failed attribs dispatch
WARNING syncd#SDK: :- collectAclCounterAttrs: Failed to get attr of ACL counter oid:0x9000000001680: SAI_STATUS_ITEM_NOT_FOUND
ERR syncd#SDK: [GBIN_ALLOCATOR.ERR] LID 38 doesn't exist!

How I verified it
Added a UT.

Details if related

nazariig
nazariig previously approved these changes Apr 8, 2022
View reviewed changes
stepanblyschak
stepanblyschak previously approved these changes Apr 8, 2022
View reviewed changes
@prsunny
Copy link
Collaborator

prsunny commented Apr 9, 2022

@anish-n , please review

@prsunny
Copy link
Collaborator

prsunny commented Apr 9, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run coverage.Azure.sonic-swss.amd64

@mssonicbld
Copy link
Collaborator

/AzurePipelines run coverage.Azure.sonic-swss.amd64

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run coverage.Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run coverage.Azure.sonic-swss

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vadymhlushko-mlnx
[pbh] [aclorch] Addded the register and deregister functions for flex…
332ad03 
… counter before updating counter for ACL rule

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@vadymhlushko-mlnx
[pbh] [aclorch] Added UT
ee106ba 
Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@vadymhlushko-mlnx
[pbh] [aclorch] Fixed deletion flow for ACL_COUNTER_RULE_MAP, fixed r…
f0e5040 
…eview comments

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@vadymhlushko-mlnx
Fixed review comments
af190f2 
Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@vadymhlushko-mlnx
Removed time
468cfb6 
Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@vadymhlushko-mlnx vadymhlushko-mlnx force-pushed the pbh_edit_flows_acl_fix branch 2 times, most recently from 53a3cbd to 468cfb6 Compare April 18, 2022 08:08
@vadymhlushko-mlnx
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@anish-n kindly reminder to review this PR

@liat-grozovik liat-grozovik merged commit 841f003 into sonic-net:master Apr 23, 2022
judyjoseph pushed a commit that referenced this pull request Apr 25, 2022
@vadymhlushko-mlnx @judyjoseph
[pbh] [aclorch] Fixed a bug causes by updating the flow-counter value…
fc29641 
… for the PBH rule (#2226)

- What I did
Added the register and deregister functions for the flex counter before updating counters for the ACL rule.

- Why I did it
Overcome SAI error in case of flow counters disabled and then enabled.

- How I verified it
Added a UT.

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
@sumukhatv sumukhatv mentioned this pull request Apr 30, 2022
Merged
5 tasks
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
@vadymhlushko-mlnx @preetham-singh
[pbh] [aclorch] Fixed a bug causes by updating the flow-counter value…
0dc10ba 
… for the PBH rule (sonic-net#2226)

- What I did
Added the register and deregister functions for the flex counter before updating counters for the ACL rule.

- Why I did it
Overcome SAI error in case of flow counters disabled and then enabled.

- How I verified it
Added a UT.

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liat-grozovik liat-grozovik liat-grozovik approved these changes

@anish-n anish-n anish-n approved these changes

@nazariig nazariig nazariig approved these changes

@stepanblyschak stepanblyschak stepanblyschak approved these changes

@prsunny prsunny Awaiting requested review from prsunny prsunny is a code owner

Assignees
No one assigned
Labels
Included in 202111 Branch Request for 202111 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@vadymhlushko-mlnx @prsunny @liat-grozovik @mssonicbld @stepanblyschak @nazariig @anish-n @judyjoseph @dprital